Correctly handle reparenting of controls #3004
Conversation
Crash in renderer when reparenting a control.
Renamed `VisualNode.SortChildren` -> `UpdateChildren` and make it remove nodes for controls that are no longer children.
| @@ -193,9 +193,12 @@ public void SortChildren(Scene scene) | |||
| keys.Add(((long)zIndex << 32) + i); | |||
| } | |||
|
|
|||
| var toRemove = _children.ToList(); | |||
There was a problem hiding this comment.
It might be better to use a pooled list instance to reduce allocations.
There was a problem hiding this comment.
Yeah, but I figured that there's already so many allocations in the renderer that one more won't make much difference, and we can address when we address the rest of them.
There was a problem hiding this comment.
Allocating huge arrays (which List internally does) can have a huge impact since they can end up in LOH.
There was a problem hiding this comment.
TBH it would require to have around 20k children to end up on LOH. Although we already have renderer objects in LOH (dirty visuals). We also reallocate all nodes, children, etc. per render. Maybe we should create new task for tracking potential renderer refactor.
What does the pull request do?
#2982 describes how a crash can occur in the deferred renderer when a control is reparented. Fix that crash by renaming
VisualNode.SortChildren->UpdateChildrenand make it remove nodes for controls that are no longer children.Checklist
Fixed issues
Fixes #2982